Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slightly reduce the Depth3Test that often times out on Linux #90777

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

trylek
Copy link
Member

@trylek trylek commented Aug 17, 2023

This is one of the regression tests I introduced as part of implementing protection against infinite generic recursion in Crossgen2. Turns out that on the relatively slow Helix Linux VMs the test often times out due to compilation time exceeding 10 minutes. I propose slightly trimming down the test so that it uses less time in Helix.

Thanks

Tomas

/cc @dotnet/crossgen-contrib, @dotnet/runtime-infrastructure

@trylek trylek added this to the 9.0.0 milestone Aug 17, 2023
@ghost ghost assigned trylek Aug 17, 2023
@ivdiazsa
Copy link
Member

ivdiazsa commented Aug 17, 2023

Do we know if this is a natural problem with not-too-fast machines, or whether we have an unnatural slowdown we ought to investigate and fix?

@ivdiazsa ivdiazsa added test-failure os-linux Linux OS (any supported distro) labels Aug 17, 2023
@trylek
Copy link
Member Author

trylek commented Aug 17, 2023

Well, in #87532 I proposed a minor optimization in the algorithm but the PR was abandoned as Michal pointed out that this is more or less a pathological case we don't need to optimize for. In a similar vein, on my quite fast devbox the test in checked mode takes about 1 minute, I can easily imagine it can be 10 times slower on a shared Azure VM.

@ivdiazsa
Copy link
Member

Well, in #87532 I proposed a minor optimization in the algorithm but the PR was abandoned as Michal pointed out that this is more or less a pathological case we don't need to optimize for. In a similar vein, on my quite fast devbox the test in checked mode takes about 1 minute, I can easily imagine it can be 10 times slower on a shared Azure VM.

I see. Well, in that case it LGTM! I've restarted the failing pipeline because it seems entirely unrelated to this change. So unless @MichalStrehovsky has further feedback, I think we can merge this, assuming that failure is indeed unrelated like I suspect it to be.

This is one of the regression tests I introduced as part of
implementing protection against infinite generic recursion in
Crossgen2. Turns out that on the relatively slow Helix Linux
VMs the test often times out due to compilation time exceeding
10 minutes. I propose slightly trimming down the test so that
it uses less time in Helix.

Thanks

Tomas
@trylek
Copy link
Member Author

trylek commented Aug 22, 2023

Thanks Andy and Ivan for reviewing. I have rebased the change against latest main and I have added the removal of the clause for Depth3Test from issues.targets where Bruce added it last week to baseline the relatively frequent lab failure. I don't think it's necessary to port this basically negative test to .NET 8 RC1 - let me know if you think otherwise.

@ivdiazsa
Copy link
Member

Wasm failure with Mono is a known infrastructure issue unrelated to this change, so I'll be merging this right now.

@ivdiazsa ivdiazsa merged commit d7138f3 into dotnet:main Aug 22, 2023
99 of 101 checks passed
@trylek trylek deleted the Depth3TestReduction branch September 5, 2023 19:27
@ghost ghost locked as resolved and limited conversation to collaborators Oct 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants